Speed Up Sync Cog Loading#2148
Conversation
The user syncer was blocking the startup of the sync cog due to having to perform thousands of pointless member fetch requests. This speeds up that process by increasing the probability that the cache is up-to-date using `Guild.chunked`, and limiting the fetches to members who were in the guild during the previous sync only. Co-authored-by: ChrisJL <chrislovering@users.noreply.github.com> Co-authored-by: wookie184 <wookie1840@gmail.com> Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
HassanAbouelela
added
t: bug
| log.info("Waiting for guild to be chunked to start syncers.") | ||
| while not guild.chunked: | ||
| await asyncio.sleep(10) | ||
| log.info("Starting syncers.") |
There was a problem hiding this comment.
Is this guaranteed to succeed? If not, there should be some time out and the syncer should still run if possible.
There was a problem hiding this comment.
The only time this won't succeed is if the member cache isn't populated by Discord.py, which could happen in a very rare case if Discord has gateway issues.
In that case we wouldn't want the user syncer to run at all, as we assume further down if a user isn't in the member cache, they are no longer a member.
There was a problem hiding this comment.
Does a 30 minute timeout sound reasonable?
There was a problem hiding this comment.
30 minutes would be ample time for this to succeed.
in the case where this times out I think the correct thing to do would be to log a warning/error and cancel the cog load.
There was a problem hiding this comment.
It doesn't sound like the syncer should run in that case. Thus, it might as well wait forever, unless you think some log message would be useful. I think wait_until_guild_available already checks if the caches are populated anyway (I assume the syncer cog is waiting for that somewhere).
There was a problem hiding this comment.
Added an exception and timeout in acc1654. It will now log the following after 30 minutes:
discord.ext.commands.errors.ExtensionFailed: Extension 'Sync' raised an error: RuntimeError: The guild was not chunked in time, not loading sync cog.
MarkKoz
left a comment
MarkKoz
left a comment
There was a problem hiding this comment.
What if the member was not in the cache and in_guild = False. Does this mean the DB won't be able to know that they've rejoined the server (until the next sync where hopefully they are in cache)?
|
The alternative would be fetching thousands of members (up until it was purged 200-300k). That doesn't seem feasible anymore. |
|
What kind of impact could a member being out of sync have on the rest of our features? |
|
I think the big impact would primarily be on moderation tools. More specifically, the user embed would be off. There are some other operations that would have problems if the cache was off (banning would raise an error for instance), but that is only in the case where the user is not in the guild, but in the cache. I don't think that's an anticipated scenario, the opposite is more likely. |
Adds a 30-minute timeout while waiting for the guild to be chunked in the sync cog, after which the cog is not loaded. Signed-off-by: Hassan Abouelela <hassan@hassanamr.com>
3 participants
The user syncer was blocking the startup of the sync cog due to having to perform thousands of pointless member fetch requests. This speeds up that process b:
Guild.chunked, and assuming that any found cache data is correct